Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PostgreSQL: get_sla_ok_percent to return decimal #710

Merged
merged 1 commit into from
Mar 25, 2024
Merged

PostgreSQL: get_sla_ok_percent to return decimal #710

merged 1 commit into from
Mar 25, 2024

Conversation

oxzi
Copy link
Member

@oxzi oxzi commented Mar 19, 2024

The final division within the get_sla_ok_percent SQL function in its PostgreSQL implementation silently truncated decimal places. An explicit decimal cast resulted for this equation resulted in a decimal value.

To both verify and catch this in the future, a test with odd numbers was added. This already succeeded for MySQL, but needed the modified schema for PostgreSQL.

  • Test against the old schema: 
    + ICINGADB_TESTS_DATABASE_TYPE=mysql
+ ICINGA_TESTING_ICINGADB_BINARY=[...]/git/github.com/Icinga/icingadb/icingadb
+ ICINGA_TESTING_ICINGADB_SCHEMA_MYSQL=[...]/git/github.com/Icinga/icingadb/schema/mysql/schema.sql
+ ICINGA_TESTING_ICINGADB_SCHEMA_PGSQL=[...]/git/github.com/Icinga/icingadb/schema/pgsql/schema.sql
+ ./icingadb-test -icingatesting.debuglog debug.log -test.v -test.run '^\QTestSla\E$/^\QMultipleStateChangesOddNumbers\E$'
=== RUN   TestSla
=== RUN   TestSla/MultipleStateChangesOddNumbers
=== RUN   TestSla/MultipleStateChangesOddNumbers/Host
=== RUN   TestSla/MultipleStateChangesOddNumbers/Service
--- PASS: TestSla (7.06s)
    --- PASS: TestSla/MultipleStateChangesOddNumbers (0.02s)
        --- PASS: TestSla/MultipleStateChangesOddNumbers/Host (0.01s)
        --- PASS: TestSla/MultipleStateChangesOddNumbers/Service (0.01s)
PASS
+ ICINGADB_TESTS_DATABASE_TYPE=pgsql
+ ICINGA_TESTING_ICINGADB_BINARY=[...]/git/github.com/Icinga/icingadb/icingadb
+ ICINGA_TESTING_ICINGADB_SCHEMA_MYSQL=[...]/git/github.com/Icinga/icingadb/schema/mysql/schema.sql
+ ICINGA_TESTING_ICINGADB_SCHEMA_PGSQL=[...]/git/github.com/Icinga/icingadb/schema/pgsql/schema.sql
+ ./icingadb-test -icingatesting.debuglog debug.log -test.v -test.run '^\QTestSla\E$/^\QMultipleStateChangesOddNumbers\E$'
=== RUN   TestSla
=== RUN   TestSla/MultipleStateChangesOddNumbers
=== RUN   TestSla/MultipleStateChangesOddNumbers/Host
    sla_test.go:290: 
        	Error Trace:	[...]/git/github.com/Icinga/icingadb/tests/sql/sla_test.go:290
        	            				[...]/git/github.com/Icinga/icingadb/tests/sql/sla_test.go:254
        	Error:      	Not equal: 
        	            	expected: 56.2
        	            	actual  : 56
        	Test:       	TestSla/MultipleStateChangesOddNumbers/Host
        	Messages:   	unexpected SLA value
=== RUN   TestSla/MultipleStateChangesOddNumbers/Service
    sla_test.go:290: 
        	Error Trace:	[...]/git/github.com/Icinga/icingadb/tests/sql/sla_test.go:290
        	            				[...]/git/github.com/Icinga/icingadb/tests/sql/sla_test.go:257
        	Error:      	Not equal: 
        	            	expected: 56.2
        	            	actual  : 56
        	Test:       	TestSla/MultipleStateChangesOddNumbers/Service
        	Messages:   	unexpected SLA value
--- FAIL: TestSla (2.60s)
    --- FAIL: TestSla/MultipleStateChangesOddNumbers (0.02s)
        --- FAIL: TestSla/MultipleStateChangesOddNumbers/Host (0.01s)
        --- FAIL: TestSla/MultipleStateChangesOddNumbers/Service (0.01s)
FAIL
  • Test against the new schema: 
    + ICINGADB_TESTS_DATABASE_TYPE=mysql
+ ICINGA_TESTING_ICINGADB_BINARY=[...]/git/github.com/Icinga/icingadb/icingadb
+ ICINGA_TESTING_ICINGADB_SCHEMA_MYSQL=[...]/git/github.com/Icinga/icingadb/schema/mysql/schema.sql
+ ICINGA_TESTING_ICINGADB_SCHEMA_PGSQL=[...]/git/github.com/Icinga/icingadb/schema/pgsql/schema.sql
+ ./icingadb-test -icingatesting.debuglog debug.log -test.v -test.run '^\QTestSla\E$/^\QMultipleStateChangesOddNumbers\E$'
=== RUN   TestSla
=== RUN   TestSla/MultipleStateChangesOddNumbers
=== RUN   TestSla/MultipleStateChangesOddNumbers/Host
=== RUN   TestSla/MultipleStateChangesOddNumbers/Service
--- PASS: TestSla (7.14s)
    --- PASS: TestSla/MultipleStateChangesOddNumbers (0.02s)
        --- PASS: TestSla/MultipleStateChangesOddNumbers/Host (0.01s)
        --- PASS: TestSla/MultipleStateChangesOddNumbers/Service (0.01s)
PASS
+ ICINGADB_TESTS_DATABASE_TYPE=pgsql
+ ICINGA_TESTING_ICINGADB_BINARY=[...]/git/github.com/Icinga/icingadb/icingadb
+ ICINGA_TESTING_ICINGADB_SCHEMA_MYSQL=[...]/git/github.com/Icinga/icingadb/schema/mysql/schema.sql
+ ICINGA_TESTING_ICINGADB_SCHEMA_PGSQL=[...]/git/github.com/Icinga/icingadb/schema/pgsql/schema.sql
+ ./icingadb-test -icingatesting.debuglog debug.log -test.v -test.run '^\QTestSla\E$/^\QMultipleStateChangesOddNumbers\E$'
=== RUN   TestSla
=== RUN   TestSla/MultipleStateChangesOddNumbers
=== RUN   TestSla/MultipleStateChangesOddNumbers/Host
=== RUN   TestSla/MultipleStateChangesOddNumbers/Service
--- PASS: TestSla (2.62s)
    --- PASS: TestSla/MultipleStateChangesOddNumbers (0.02s)
        --- PASS: TestSla/MultipleStateChangesOddNumbers/Host (0.01s)
        --- PASS: TestSla/MultipleStateChangesOddNumbers/Service (0.00s)
PASS

Closes #648.

@cla-bot cla-bot bot added the cla/signed label Mar 19, 2024
@oxzi oxzi added this to the 1.1.2 milestone Mar 19, 2024
@oxzi oxzi force-pushed the pgsql-get_sla_ok_percent-decimal-i648 branch from d077652 to c98fddd Compare March 19, 2024 14:49
@oxzi oxzi requested a review from julianbrost March 20, 2024 08:36
julianbrost
julianbrost requested changes Mar 20, 2024
View reviewed changes
tests/sql/sla_test.go Show resolved Hide resolved
schema/pgsql/upgrades/1.1.2.sql Outdated Show resolved Hide resolved
schema/pgsql/schema.sql Outdated Show resolved Hide resolved
@oxzi
PostgreSQL: get_sla_ok_percent to return decimal
34ac386 
The final division within the get_sla_ok_percent SQL function in its
PostgreSQL implementation silently truncated decimal places. An explicit
decimal cast resulted for this equation resulted in a decimal value.

To both verify and catch this in the future, a test with odd numbers was
added. This already succeeded for MySQL, but needed the modified schema
for PostgreSQL.

Closes #648.
@oxzi oxzi force-pushed the pgsql-get_sla_ok_percent-decimal-i648 branch from c98fddd to 34ac386 Compare March 20, 2024 12:16
@oxzi oxzi requested a review from julianbrost March 20, 2024 12:18
yhabteab
yhabteab approved these changes Mar 22, 2024
View reviewed changes
Copy link
Member

@yhabteab yhabteab left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bildschirmfoto 2024-03-22 um 16 52 34

↓

Bildschirmfoto 2024-03-22 um 16 52 55

@julianbrost julianbrost merged commit f0b9c59 into main Mar 25, 2024
31 checks passed
@julianbrost julianbrost deleted the pgsql-get_sla_ok_percent-decimal-i648 branch March 25, 2024 08:54
@Al2Klimov Al2Klimov mentioned this pull request Mar 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yhabteab yhabteab yhabteab approved these changes

@julianbrost julianbrost julianbrost approved these changes

Assignees
No one assigned
Labels
cla/signed
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.

get_sla_ok_percent (postgres) returns integer
3 participants
@oxzi @julianbrost @yhabteab